Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(docs-infra): change app-list-item to app-item-list #35601

Closed
wants to merge 1 commit into from
Closed

fix(docs-infra): change app-list-item to app-item-list #35601

wants to merge 1 commit into from

Conversation

sonukapoor
Copy link
Contributor

The app-list-item component sounds like it is used for a single
item, however it renders a list of items. There were also
several changes in the documentation, where it was becoming
confusing if the app-list-item is using a single item or multiple
items. This commit fixes this issue. It renames the component and its
respective properties to make sure that the intention is very clear.

Closes #35598

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #35598

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@pullapprove pullapprove bot requested a review from kara February 21, 2020 03:36
@ngbot ngbot bot added this to the needsTriage milestone Feb 22, 2020
@sonukapoor
fix(docs-infra): change app-list-item to app-item-list
672d8d2 
The `app-list-item` component sounds like it is used for a single
item, however it renders a list of items. There were also
several changes in the documentation, where it was becoming
confusing if the `app-list-item` is using a single item or multiple
items. This commit fixes this issue. It renames the component and its
respective properties to make sure that the intention is very clear.

Closes angular#35598
@sonukapoor
Copy link
Contributor Author

@gkalpak Can you review this?

@gkalpak
Copy link
Member

gkalpak commented Feb 27, 2020

@sonukapoor, this already has assigned reviewers 😉
Happy to take a look too, but first we should ensure that the content is OK (i.e. correct and makes sense for the concepts we are trying to get across), for which @kapunahelewong is much more qualified than me 😁

@ngbot ngbot bot modified the milestones: needsTriage, Backlog Feb 28, 2020
@kapunahelewong kapunahelewong added this to In Review in docs Feb 28, 2020
@mary-poppins
Copy link

You can preview 02f8428 at https://pr35601-02f8428.ngbuilds.io/.
You can preview 672d8d2 at https://pr35601-672d8d2.ngbuilds.io/.

kapunahelewong
kapunahelewong approved these changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@kapunahelewong kapunahelewong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you very much, @sonukapoor!!

@kapunahelewong kapunahelewong removed the request for review from kara February 28, 2020 16:51
@kapunahelewong
Copy link
Contributor

merge-assistance: Global approval for this docs-only change.

@kapunahelewong kapunahelewong added the action: merge The PR is ready for merge by the caretaker label Mar 3, 2020
@kapunahelewong kapunahelewong moved this from In Review to Waiting for Merge in docs Mar 3, 2020
@kapunahelewong kapunahelewong removed the action: merge The PR is ready for merge by the caretaker label Mar 3, 2020
@sonukapoor
Copy link
Contributor Author

@aikidave Can we get this merged?

kara
kara approved these changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@kara kara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kara kara added the action: merge The PR is ready for merge by the caretaker label Mar 13, 2020
@aikithoughts aikithoughts self-requested a review March 20, 2020 15:51
@kapunahelewong kapunahelewong removed the request for review from IgorMinar March 20, 2020 18:04
mhevery pushed a commit that referenced this pull request Mar 23, 2020
@sonukapoor @mhevery
fix(docs-infra): change app-list-item to app-item-list (#35601)
ba3edda 
The `app-list-item` component sounds like it is used for a single
item, however it renders a list of items. There were also
several changes in the documentation, where it was becoming
confusing if the `app-list-item` is using a single item or multiple
items. This commit fixes this issue. It renames the component and its
respective properties to make sure that the intention is very clear.

Closes #35598

PR Close #35601
@mhevery mhevery closed this in 9d415f9 Mar 23, 2020
@kapunahelewong kapunahelewong moved this from Waiting for Merge to Done in docs Mar 23, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kara kara kara approved these changes

@kapunahelewong kapunahelewong kapunahelewong approved these changes

@aikithoughts aikithoughts aikithoughts approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker aio: preview cla: yes effort1: hours risk: low target: patch This PR is targeted for the next patch release type: bug/fix
Projects
docs
Done
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

Improve Template Syntax code + documentation
8 participants
@sonukapoor @gkalpak @mary-poppins @kapunahelewong @kara @aikithoughts @mhevery @googlebot